Skip to content

Avoid over-reserving Vecs in compression codecs - #10483

Open
MassivePizza wants to merge 8 commits into
apache:mainfrom
massive-com:compress-reserve-begone
Open

Avoid over-reserving Vecs in compression codecs#10483
MassivePizza wants to merge 8 commits into
apache:mainfrom
massive-com:compress-reserve-begone

Conversation

@MassivePizza

@MassivePizza MassivePizza commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

This is a continuation to PR #10345.

Rationale for this change

The Vecs that we over-reserved can be long-lived, see issue above.

What changes are included in this PR?

Use zstd_safe contexts directly to compress/decompress, allowing us to append to Vec without reserving excessive amounts.

Remove redundant loops in LZ4Codec.

Are these changes tested?

Should be covered by existing tests.

One thing I felt while looking was the lack of tests regarding compression frames (some codecs look like they may decompress too much or not enough). Could add tests for that if desired.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the parquet Changes to the parquet crate label Jul 29, 2026
@MassivePizza
MassivePizza marked this pull request as ready for review July 29, 2026 14:57
@MassivePizza
MassivePizza marked this pull request as draft July 29, 2026 15:11
@MassivePizza
MassivePizza marked this pull request as ready for review July 29, 2026 16:07
@Jefffrey

Jefffrey commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

run benchmark compression

@adriangbot

This comment was marked as duplicate.

Sign in to view
@adriangbot

adriangbot commented Jul 30, 2026

Copy link
Copy Markdown

🤖 Arrow criterion benchmark completed (GKE) | trigger

Instance: c4a-highmem-16 (12 vCPU / 65 GiB)

CPU Details (lscpu) 
Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected
Details 

group                                               compress-reserve-begone                main
-----                                               -----------------------                ----
compress BROTLI(BrotliLevel(1)) - alphanumeric      1.00      4.2±0.01ms        ? ?/sec    1.00      4.2±0.03ms        ? ?/sec
compress BROTLI(BrotliLevel(1)) - words             1.00      5.3±0.02ms        ? ?/sec    1.00      5.2±0.00ms        ? ?/sec
compress GZIP(GzipLevel(6)) - alphanumeric          1.00     20.7±0.09ms        ? ?/sec    1.00     20.8±0.13ms        ? ?/sec
compress GZIP(GzipLevel(6)) - words                 1.00     26.1±0.06ms        ? ?/sec    1.00     26.1±0.03ms        ? ?/sec
compress LZ4 - alphanumeric                         1.00     39.5±0.40µs        ? ?/sec    1.03     40.7±0.26µs        ? ?/sec
compress LZ4 - words                                1.00   1807.5±1.40µs        ? ?/sec    1.00   1807.8±1.70µs        ? ?/sec
compress LZ4_RAW - alphanumeric                     1.00     38.9±0.47µs        ? ?/sec    1.04     40.4±0.22µs        ? ?/sec
compress LZ4_RAW - words                            1.00   1808.3±3.40µs        ? ?/sec    1.00   1807.6±1.36µs        ? ?/sec
compress SNAPPY - alphanumeric                      1.02     34.1±0.47µs        ? ?/sec    1.00     33.4±0.12µs        ? ?/sec
compress SNAPPY - words                             1.00   1459.3±3.19µs        ? ?/sec    1.00   1459.2±1.19µs        ? ?/sec
compress ZSTD(ZstdLevel(1)) - alphanumeric          1.03    953.8±1.83µs        ? ?/sec    1.00    923.2±0.50µs        ? ?/sec
compress ZSTD(ZstdLevel(1)) - words                 1.05      2.8±0.01ms        ? ?/sec    1.00      2.7±0.01ms        ? ?/sec
decompress BROTLI(BrotliLevel(1)) - alphanumeric    1.00      7.3±0.01ms        ? ?/sec    1.00      7.3±0.03ms        ? ?/sec
decompress BROTLI(BrotliLevel(1)) - words           1.01      3.3±0.01ms        ? ?/sec    1.00      3.2±0.01ms        ? ?/sec
decompress GZIP(GzipLevel(6)) - alphanumeric        1.00      2.8±0.00ms        ? ?/sec    1.01      2.8±0.01ms        ? ?/sec
decompress GZIP(GzipLevel(6)) - words               1.00    969.3±6.56µs        ? ?/sec    1.03   1000.0±7.59µs        ? ?/sec
decompress LZ4 - alphanumeric                       1.00     37.1±0.09µs        ? ?/sec    1.04     38.6±1.09µs        ? ?/sec
decompress LZ4 - words                              1.00    327.4±0.63µs        ? ?/sec    1.01    332.1±0.30µs        ? ?/sec
decompress LZ4_RAW - alphanumeric                   1.00     37.1±0.08µs        ? ?/sec    1.03     38.2±0.26µs        ? ?/sec
decompress LZ4_RAW - words                          1.00    327.3±0.23µs        ? ?/sec    1.01    330.5±0.27µs        ? ?/sec
decompress SNAPPY - alphanumeric                    1.01     26.9±0.37µs        ? ?/sec    1.00     26.7±0.36µs        ? ?/sec
decompress SNAPPY - words                           1.00    580.8±0.86µs        ? ?/sec    1.00    580.8±0.85µs        ? ?/sec
decompress ZSTD(ZstdLevel(1)) - alphanumeric        1.04   1048.4±3.54µs        ? ?/sec    1.00   1003.9±0.82µs        ? ?/sec
decompress ZSTD(ZstdLevel(1)) - words               1.03   1014.0±1.13µs        ? ?/sec    1.00    979.9±0.92µs        ? ?/sec

Resource Usage

base (merge-base)

Metric Value
Wall time 305.1s
Peak memory 14.6 MiB
Avg memory 12.7 MiB
CPU user 297.8s
CPU sys 1.2s
Peak spill 0 B

branch

Metric Value
Wall time 310.1s
Peak memory 14.8 MiB
Avg memory 13.0 MiB
CPU user 305.2s
CPU sys 1.2s
Peak spill 0 B

File an issue against this benchmark runner

mapleFU
mapleFU reviewed Jul 30, 2026
View reviewed changes
Comment thread parquet/src/compression.rs
if to_flush == 0 {
break;
}
output_buf.reserve_exact(to_flush);

@mapleFU mapleFU Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emmm calling reserve_exact in a loop is a bit weird, does this guranteed calling once?

@MassivePizza MassivePizza Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not ideal but that's what the Zstd API looks like. end_stream will essentially always produce N followed by 0 bytes. Only multi-threaded encode would need multiple flushes as workers drain.

Just reserve works but I tried to avoid need for future shrinks.
From testing, this is often the first and last alloc too (output_buf Vecs don't seem to be reused much).

@MassivePizza

MassivePizza commented Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Did a (hopefully final) set of changes that should make both benches and memory usage happy.
I did have to enable the experimental zstd feature, but that's hopefully fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mapleFU mapleFU mapleFU left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

parquet Changes to the parquet crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Parquet: Should DataPageV2 shrink_to_fit?

4 participants

@MassivePizza @Jefffrey @adriangbot @mapleFU